🔀 Branch Merge Check

PR direction: feature/portfolio-monitor → dev

✅ Passed — feature/portfolio-monitor → dev is a valid merge direction

✅ Community PR Path Check — Passed

All changed files are inside the community/ folder. Looks good!

✅ Ability Validation Passed

📋 Validating: community/portfolio-monitor
  ✅ All checks passed!

🔍 Lint Results

✅ __init__.py — Empty as expected

Files linted: community/portfolio-monitor/background.py community/portfolio-monitor/main.py

✅ Flake8 — Passed

✅ All checks passed!

Hey @uzair401 - appreciate the thorough review, really helpful catching these before merge.

Went through everything you flagged and pushed fixes for all of it:

The LLM hallucination issue was the biggest one - _handle_add, _handle_check, and _handle_set_alert are all conversational now with explicit while True loops and exit detection, so the user stays in the skill for follow-ups instead of falling back to the agent. Also dropped the "stock" not in t exclusion and expanded the hotwords so routing is more reliable.

Switched the whole persistence layer over to Context Storage (get_single_key / create_key / update_key) in both files. The old file I/O approach was definitely the root cause of stocks not surviving across triggers.

API keys are now loaded at runtime via get_api_keys() - nothing hardcoded. Added an upfront check in _run() that tells the user exactly what to do if the Finnhub key is missing, and updated the README to point to Settings → API Keys instead of telling people to edit source files.

Fixed all four of the silent correctness bugs - the dead P&L expression, the pos_value vs pos_cost denominator, the _handle_set_alert reset issue (now uses setdefault(ticker, {}) + is not None), and does_match no longer touches the LLM at all (static map + regex only).

On the daemon side: EOD summary now forces a fresh fetch before computing totals, _handle_portfolio does TTL-aware cache refreshes (not just missing-key checks), _handle_check saves the cache after a fresh fetch, and MAX_API_CALLS_PER_POLL is raised to 50 so larger portfolios don't silently stall.

Let me know if anything else needs attention.

Hey @uzair401 - pushed another round of changes based on your second review.

LLM intent router is in. _classify_intent now routes through text_to_text_response with a clear prompt covering all 8 intents. The only thing still handled by regex is CLEAR and REMOVE - those are unambiguous and destructive, so there's no reason to burn an LLM call on them. Everything else ("what happened to my Google position", "how did my stocks do today") gets routed correctly now.

Day-over-day compare is a proper feature now. Saying "compare my stocks", "day over day", or "versus yesterday" triggers _handle_compare, which reads prev_close from the cached quote data and speaks each holding's move - direction, percent, and dollar change. Added the trigger phrases to HOTWORDS and the README.

Feature-centric entry point - _handle_portfolio is now a hub. First thing you hear is a tight two-line snapshot: total value, today's P&L, overall P&L. Then it prompts: say 'breakdown' for full detail, 'compare' for day-over-day, 'movers', or a stock name. The full per-stock breakdown is still there - just moved to _speak_portfolio_breakdown and called on demand. This way the first response is fast and you can drill into whatever you actually want.

Let me know how testing goes.

Thanks @hassan1731996 for the quick turnaround on the requested changes. There are still a few improvements that need to be made, and I'll keep the PR open until those changes are in. I'll work through these, and once they're finalized by our reviewer and QA team, I'll share an update along with the revised code and updated README. After that, we'll proceed with marketplace approval. Thanks again for contributing to OpenHome!

Hey @uzair401 - glad the changes landed well. Happy to keep iterating if there's anything specific you want me to adjust on my end in the meantime. Looking forward to seeing what the QA pass surfaces - and appreciate you taking it the rest of the way to marketplace. Exciting milestone for this one.

Few more additions in this push:

UPDATE intent - full position management in one flow: bought more shares (recalculates weighted average cost), sold some (reduces share count, or removes the stock entirely if you sell all of it with confirmation), and correct/overwrite if the numbers are just wrong. Reachable directly via voice ("I bought more Tesla", "sold some Apple") or from within the ADD flow when a stock already exists.

MARKET intent - "how are the markets" pulls live S&P 500, Nasdaq, and Dow Jones from the same Finnhub feed. Works both as a standalone trigger and as a navigation option inside the portfolio hub.

Hub navigation → LLM sub-router - unified the portfolio dashboard's follow-up loop with the same LLM routing approach used at the top level. Both now go through the same pattern so phrasing is handled consistently throughout.

Chunked voice output - BREAKDOWN and COMPARE speak 4 stocks at a time and ask "Want to hear the rest?" before continuing. Keeps things natural for larger portfolios instead of reading everything out at once.

README example conversation - added a full walkthrough: portfolio hub, market check, per-stock query, breakdown pagination, position update, and a background alert firing.

Let me know if you have any other thoughts.